-
Notifications
You must be signed in to change notification settings - Fork 11
ENH: Centralize fMRIPrep's and MRIQC's guidelines for Docker & DataLad #47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
efb2d56
to
82de14f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! More of comments and questions rather than suggestions. Overall, I think there is need and opportunity to reference/integrate with https://github.com/ReproNim/containers efforts to simplify execution for those who can/are using datalad already.
@@ -18,6 +12,12 @@ on this documentation page will refer to it. | |||
|
|||
## *DataLad* and *Docker* | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could/should I propose (separate PR on top?) in this section and/or https://github.com/nipreps/nipreps.github.io/blob/HEAD/docs/apps/singularity.md file to mention our https://github.com/ReproNim/containers which contains (automatically updates) pre-created singularity images for all bids-apps (thus including fmriprep, mriqc), and providing some helpers to streamline their use and more guaranteed reproducibility (isolated environment execution etc).
Note that wrapper also tries to support non-Linux systems (OSX) where we could run singularity under docker. Or could also be used on Linux if there is no singularity installation.
https://github.com/ReproNim/containers?tab=readme-ov-file#runnable-script provides a "typical" use example based on mriqc.
https://github.com/OpenNeuroDerivatives/ by @jbwexler (and @effigies ?) use that ReproNim/containers as a subdatset archive of the images with reproman run
for execution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also that should include YODA aspects whenever talking about containers... with them it becomes possible to encapsulate all digital objects nicely and reproducibly (there is no guarantee that docker:// would later give you the images used etc)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes! Mentioning ReproNim on a separate PR would be fantastic.
Co-authored-by: Yaroslav Halchenko <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the time and the valuable comments :)
@@ -18,6 +12,12 @@ on this documentation page will refer to it. | |||
|
|||
## *DataLad* and *Docker* | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes! Mentioning ReproNim on a separate PR would be fantastic.
44360cb
to
c66ae8e
Compare
Co-authored-by: Yaroslav Halchenko <[email protected]>
Related-to: nipreps/nipreps.github.io#47 (comment) Co-authored-by: Yaroslav Halchenko <[email protected]>
Related-to: nipreps/nipreps.github.io#47 (comment) Co-authored-by: Yaroslav Halchenko <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestions on docker.md.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only a small suggestion for datalad.
|
||
**Memory will be a common culprit** when working with large datasets | ||
(+10GB). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
another shameless plug which might be of interest/help.
Inspired by our reproman, and BrainLife's helper to monitor execution of compute, we created a simple helper https://github.com/con/duct which could be of help to monitor/identify memory and cpu requirements for e.g. future informed specification for job parameters or plotting resource consumption during compute. It also takes care about storing stdout/stderr outputs produced, thus making it possible (if used along with datalad *run
) to capture those for possible future troubleshooting etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you finally send an additional PR, I'm happy to see this documented there :)
-v path/to/data:/data:ro \ | ||
-v path/to/output:/out \ | ||
-v path/to/work:/work \ # mount from host | ||
nipreps/fmriprep:<latest-version> \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw -- isn't there freesurfer
license needed to be passed somehow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Further up, on line 18, it says that all examples will refer to MRIQC (which doesn't require freesurfer license).
nvm, didn't realize this was the docker.md file, not the datalad.md file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I had the freesurfer license issue in mind. At the moment it's defined in fMRIPrep's documentation but we probably want to bubble it up here.
Co-authored-by: Chris Markiewicz <[email protected]> Co-authored-by: Yaroslav Halchenko <[email protected]>
Related-To: https://github.com/nipreps/nipreps.github.io/pull/47/files#r1904836364 Co-Authored-By: McKenzie P. Hagen <[email protected]>
Co-authored-by: Chris Markiewicz <[email protected]>
|
||
By default, Docker will run the container with the | ||
user id (uid) **0**, which is reserved for the default **root** | ||
account in *Linux*. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless the actual docker there is podman
which by default would run rootless ;)
ha@hopa:~$ rm -f 123; docker run -it -v $PWD:$PWD -w $PWD --entrypoint bash nipreps/fmriprep:latest -c 'echo `whoami` am in; id; pwd; touch 123; ls -l 123' ; echo "`whoami` is out"; ls -ld 123
Emulate Docker CLI using podman. Create /etc/containers/nodocker to quiet msg.
root am in
uid=0(root) gid=0(root) groups=0(root)
/home/ha
-rw-r--r-- 1 root root 0 Jan 7 14:19 123
ha is out
-rw-r--r-- 1 ha ha 0 Jan 7 09:19 123
I am yet to try podman on hpc/for hpc, using primarily for services, but googled into https://github.com/NERSC/podman-hpc ... worth researching and at least pointing users that there is another OCI solution podman which might give them easier means to run compute on their infrastructure.
Updates the documentation from MRIQC and fMRIPrep with the goal of centralizing containerized execution. The end goal is to link these guidelines from fMRIPrep and MRIQC instead of maintaining slightly varying different versions.
This also builds on a previous addition regarding DataLad guidelines, raised by @mckenziephagen and @arokem from comments in the review of MRIQC's protocol paper.
Pinging @effigies, @mgxd, @yarikoptic, @tsalo for general feedback.